feat: add proper wrapping for esm and Deno by updating dependencies #143
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
- add esm test to npm run script - run full tests from esm Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com>
shadowspawn
force-pushed
the
feature/esm-wrapping
branch
from
cba92da
to
3cfe03d
Compare
|
I'm happy to switch back from my fork once it has the same behavior on esm as cjs, whether it's the pr I sent or this one or some other one. |
|
I was concerned the approach with aliases in #139 and used in jackspeak triggered multiple problem reports from users of yarn. We are using rollup though various yargs repos to help with the hybrid cjs/esm, so already have a tool for the job. Reports of problems with yarn 1:
And one problem with yarn 3: |
This comment was marked as outdated.
This comment was marked as outdated.
|
There was a clash between versions of TypeScript from 4.7 and rollup@2 that was fixed in rollup@3.0.0, but that rollup requires node 14. Pinning back TypeScript to 4.6.4 works around the problem. (Now, what is going wrong with that coverage check...) |
This comment was marked as outdated.
This comment was marked as outdated.
|
There is absolutely no reason for any module to keep supporting node 14, let alone node 12. (For that matter, I obviously don't see the point in trying to support yarn 1, but here we are.) If someone is using EOL versions of node, they are already off the supported landscape, and can be expected to either very carefully lock down everything, or be broken repeatedly. |
Yes, I am just trying to work with the current constraints so this PR is independent of a bumping minimum node to (say) 18. (And freshly reminded of what a pain that can be.) |
|
Just wanted to say that I appreciate the effort put into this, as it helps immensely in transitioning fully to ESM. The fork of this package used in |
This is yet another approach for adding wrapping for ESM and Deno, see also #119 #139 #140 !
Problem
Only CJS currently gets smart wrapping (#89 #113 #138 yargs/yargs#2112)
Solution
Upgrade to newer ansi dependencies which are ESM and use rollup to transpile the CJS version.
test, run both cjs and esm tests togetherNotes for interest of reviewers:
package.json. I likepackage.jsonso have a single source of truth for dependency versions, but I don't know the tradeoffs vs using an URL to a CDN, and if it works ok when published to deno.land.rollup@3as it requires node 14. (And cliui still only requires node 12.)string-width@6as it requires requires node 16.